-
-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make creating RequestInfo
backwards compatible with 3.10
#9873
Conversation
It was unexpected that this object was being created directly outside of aiohttp internals. While it looks like its only used for mocking downstream, we can accomodate that by subclassing the NamedTuple and providing a `__new__` while keeping the faster `tuple.__new__` internally. fixes #9866
Is it for Anyway, the perfect example of innocent change that breaks backward compatibility. I did many such commits in the past, and maybe will do in the future again. |
Yes. We can also take the position as since they are mocking internals we won't fix it (which was my initial call), but it seems like pnuckowski/aioresponses#262 is moving slow, and realistically they don't have a better way to mock this right now. |
CodSpeed Performance ReportMerging #9873 will not alter performanceComparing Summary
|
Since restoring the backward compatibility is easy, let's do it. For more complex case "won't fix" would be an option. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #9873 +/- ##
=======================================
Coverage 98.71% 98.71%
=======================================
Files 118 118
Lines 36294 36309 +15
Branches 4315 4316 +1
=======================================
+ Hits 35826 35841 +15
Misses 315 315
Partials 153 153
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Third-party that monkey-patches aiohttp is in the danger zone by definition, though. |
Agreed. In this case, I didn't expect someone would be creating these externally... but since its not in a protected namespace, I should have assumed otherwise (and someone always does anyways even if it is) |
Yeah, users sometimes use libraries in very unpredictable way :) |
Backport to 3.11: 💚 backport PR created✅ Backport PR branch: Backported as #9874 🤖 @patchback |
(cherry picked from commit c9698c8)
…rds compatible with 3.10 (#9874) Co-authored-by: J. Nick Koston <[email protected]> fixes #9866
Backport to 3.12: 💚 backport PR created✅ Backport PR branch: Backported as #9886 🤖 @patchback |
(cherry picked from commit c9698c8)
…rds compatible with 3.10 (#9886) Co-authored-by: J. Nick Koston <[email protected]> fixes #9866
It was unexpected that this object was being created directly outside of
aiohttp
internals. While it looks like its only used for mocking internal state downstream, we can accommodate that by subclassing theNamedTuple
and providing a__new__
while keeping the fastertuple.__new__
internally.We might remove the back-compat later in 4.x development cycle, however since its currently being used downstream to mock internals and there isn't a better prescribed way to do that, leave it for now.
fixes #9866